Implement fs.glob, fs.globSync, and fs/promises.glob#6378
Implement fs.glob, fs.globSync, and fs/promises.glob#6378fraidev wants to merge 5 commits intocloudflare:mainfrom
Conversation
Implements the Node.js fs.glob APIs that were previously stubbed with ERR_UNSUPPORTED_OPERATION. Uses a custom glob-to-regex pattern matcher instead of the minimatch library which is unavailable in the Workers runtime. Closes cloudflare#5416
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
Hey @fraidev! thank you for this! I've done a first pass look over and have asked the AI agent to review also.. it should post it's findings in a moment. How closely does this follow the node.js implementation? (I haven't looked at it in a while) Is it a straight up port of the original or did you reimplement? The main reason I ask is that if it is a straight port over, we might need to carry over the Node.js copyright/license detail in the header of the new file(s). |
jasnell
left a comment
jasnell
left a comment
There was a problem hiding this comment.
Review Summary
Note: This review was generated by an AI assistant (Claude) and may contain inaccuracies. Please evaluate each suggestion on its merits.
Findings (6):
- [HIGH]
excludetype handling — unsafe cast incompileExcludewhenexcludeis a user function - [MEDIUM]
**in middle of exclude pattern doesn't match zero segments (a/**/bwon't matcha/b) - [MEDIUM]
walkGlobhas unbounded recursion depth — no depth guard - [MEDIUM] Non-null assertions (
!) used extensively — violates ts-style convention - [MEDIUM] Missing callback guard in
fs.glob— throws when callback omitted - [LOW] Copyright year says 2017-2022 in new file
Hey @jasnell! This is a reimplementation, not a port. Node.js uses |
|
@jasnell is there a way to run CI for a non-Cloudflare member? |
fraidev
changed the title
|
Just did. Internal_build will fail but I'll run that one manually |
Great! I fixed ESLint errors. I think that we need one more run. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6378 +/- ##
==========================================
- Coverage 70.80% 70.65% -0.16%
==========================================
Files 424 425 +1
Lines 115290 116825 +1535
Branches 18751 18880 +129
==========================================
+ Hits 81629 82538 +909
- Misses 22429 23043 +614
- Partials 11232 11244 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
Implements the Node.js fs.glob APIs that were previously stubbed with ERR_UNSUPPORTED_OPERATION. Uses a custom glob-to-regex pattern matcher.
Closes #5416